feat: Betterstack Importer #2016
Conversation
Signed-off-by: Moulik Aggarwal <qwertymoulik@gmail.com>
Signed-off-by: Moulik Aggarwal <qwertymoulik@gmail.com>
|
@aggmoulik is attempting to deploy a commit to the OpenStatus Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
2 issues found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/importers/src/providers/betterstack/provider.ts">
<violation number="1" location="packages/importers/src/providers/betterstack/provider.ts:113">
P2: The monitor-groups fetch + mapping block is copy-pasted identically in both branches of the `if (statusPages.length > 0)` / `else`. Move it after the conditional to eliminate the duplication — `pageId` is already in scope.</violation>
</file>
<file name="packages/api/src/service/import.ts">
<violation number="1" location="packages/api/src/service/import.ts:41">
P2: Unknown provider names silently fall through to `createStatuspageProvider()`. Separate the `default` case to throw an error for unrecognized providers, so misconfigurations fail fast instead of producing confusing downstream errors.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Signed-off-by: Moulik Aggarwal <qwertymoulik@gmail.com>
Signed-off-by: Moulik Aggarwal <qwertymoulik@gmail.com>
Signed-off-by: Moulik Aggarwal <qwertymoulik@gmail.com>
There was a problem hiding this comment.
Pull request overview
Adds a new Better Stack Uptime importer to @openstatus/importers and wires it through the API import pipeline and Dashboard UI so users can preview/run imports (including monitors) from Better Stack into OpenStatus.
Changes:
- Introduces a new Better Stack provider (Zod API types, client w/ pagination, mappers, orchestration, fixtures, and tests).
- Extends the API import pipeline to support a
betterstackprovider, monitor imports, and monitor plan-limit warnings. - Updates Dashboard import form and icons to support Better Stack-specific inputs and toggles.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/importers/src/providers/betterstack/api-types.ts | Zod schemas + TS types for Better Stack API resources. |
| packages/importers/src/providers/betterstack/client.ts | Better Stack HTTP client (Bearer auth, JSON:API-style pagination). |
| packages/importers/src/providers/betterstack/client.test.ts | Tests for Better Stack client (auth, pagination, errors, parsing). |
| packages/importers/src/providers/betterstack/fixtures.ts | Shared mock payloads for Better Stack provider tests. |
| packages/importers/src/providers/betterstack/index.ts | Barrel exports for the Better Stack provider package entry. |
| packages/importers/src/providers/betterstack/mapper.ts | Mapping functions from Better Stack shapes to OpenStatus insert shapes. |
| packages/importers/src/providers/betterstack/mapper.test.ts | Unit tests for all Better Stack mappers. |
| packages/importers/src/providers/betterstack/provider.ts | Provider orchestration (validate + phase generation). |
| packages/importers/src/providers/betterstack/provider.test.ts | Provider integration tests (phase structure, filtering, resources). |
| packages/importers/src/index.ts | Registers betterstack in IMPORT_PROVIDERS and re-exports. |
| packages/importers/package.json | Adds ./betterstack and ./betterstack/fixtures export paths. |
| packages/importers/README.md | New package-level docs incl. provider architecture and phase mapping. |
| packages/icons/src/betterstack.tsx | Adds Better Stack icon component. |
| packages/icons/src/index.tsx | Exports the new Better Stack icon. |
| packages/api/src/service/import.ts | Adds provider routing, monitor limit warnings, and monitor write phase. |
| packages/api/src/router/import.ts | Accepts betterstack provider + Better Stack-specific inputs and options. |
| apps/dashboard/src/components/forms/components/form-import.tsx | UI support for Better Stack selection, token help text, status page id, and monitor toggle. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 3. Monitor count limit | ||
| const monitorsPhase = summary.phases.find((p) => p.phase === "monitors"); | ||
| if (monitorsPhase && monitorsPhase.resources.length > 0) { | ||
| const maxMonitors = config.limits.monitors; | ||
| const existingMonitors = await db | ||
| .select() | ||
| .from(monitor) | ||
| .where(eq(monitor.workspaceId, config.workspaceId)) | ||
| .all(); | ||
| const remaining = maxMonitors - existingMonitors.length; | ||
| if (remaining <= 0) { | ||
| summary.errors.push( | ||
| `Monitor limit reached (${maxMonitors}). Upgrade your plan to import monitors.`, | ||
| ); | ||
| } else if (monitorsPhase.resources.length > remaining) { | ||
| summary.errors.push( | ||
| `Only ${remaining} of ${monitorsPhase.resources.length} monitors can be imported due to plan limit (${maxMonitors}).`, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
The new monitor limit warning logic in addLimitWarnings() isn't covered by the existing packages/api/src/service/import.test.ts suite (which already tests components/custom-domain/subscribers). Adding tests for the monitors phase warnings (under/over limit, existing monitors) would help prevent regressions.
packages/api/src/service/import.ts
Outdated
| @@ -182,6 +237,18 @@ export async function runImport(config: { | |||
| phase.status = "skipped"; | |||
| } | |||
| break; | |||
There was a problem hiding this comment.
When targetPageId is falsy (no page created and no pageId provided), the monitorGroups/componentGroups case currently does nothing unless includeComponents === false. That leaves the phase in its original "completed" state even though nothing was written. Explicitly mark the phase as skipped (and ideally update resource statuses) when targetPageId is missing.
packages/api/src/service/import.ts
Outdated
| } else if (config.options?.includeComponents === false) { | ||
| phase.status = "skipped"; |
There was a problem hiding this comment.
Similar to the monitorGroups case: if targetPageId is not set, the sections phase will silently do nothing (unless includeComponents === false), leaving the phase reported as completed. Set phase.status = "skipped" (and update resource statuses) when there is no page to attach groups/sections to.
| } else if (config.options?.includeComponents === false) { | |
| phase.status = "skipped"; | |
| } else if ( | |
| config.options?.includeComponents === false || | |
| !targetPageId | |
| ) { | |
| phase.status = "skipped"; | |
| for (const resource of phase.resources) { | |
| resource.status = "skipped"; | |
| } |
| // Monitor Groups → Component Groups (always fetched regardless of status pages) | ||
| const monitorGroups = await client.getMonitorGroups(); | ||
| const groupResources: ResourceResult[] = monitorGroups.map((g) => ({ | ||
| sourceId: g.id, | ||
| name: g.attributes.name, | ||
| status: "created" as const, | ||
| data: mapMonitorGroup(g, config.workspaceId, pageId), | ||
| })); | ||
| phases.push({ | ||
| phase: "monitorGroups", | ||
| status: "completed", | ||
| resources: groupResources, | ||
| }); | ||
|
|
||
| // Phase 5: Incidents → Status Reports | ||
| const incidents = await client.getIncidents(); | ||
| const incidentResources: ResourceResult[] = incidents.map((inc) => ({ | ||
| sourceId: inc.id, | ||
| name: inc.attributes.name ?? `Incident ${inc.id}`, | ||
| status: "created" as const, | ||
| data: mapIncidentToStatusReport(inc, config.workspaceId, pageId), | ||
| })); | ||
| phases.push({ | ||
| phase: "incidents", | ||
| status: "completed", | ||
| resources: incidentResources, | ||
| }); |
There was a problem hiding this comment.
run() always emits an incidents phase even when no page context exists (config.pageId is undefined and betterstackStatusPageId filtering yields no status page). In that situation the service layer won't be able to write incidents, but the phase will still be reported as completed. Consider only emitting page-scoped phases (sections/monitorGroups/incidents) when you have a page to attach to, or mark them as skipped with a clear reason in the summary.
| const maxMonitors = limits.monitors; | ||
| const remaining = maxMonitors - existingMonitors.length; | ||
|
|
||
| if (remaining <= 0) { |
There was a problem hiding this comment.
If remaining <= 0, writeMonitorsPhase sets phase.status = "failed" and returns, but it does not update individual resource.status values. Since importer providers initially mark resources as created, this can leave the summary claiming monitors were created when none were written. Mark all monitor resources as skipped/failed (with an explanatory resource.error) before returning so the summary accurately reflects what happened.
| if (remaining <= 0) { | |
| if (remaining <= 0) { | |
| for (const resource of phase.resources) { | |
| resource.status = "skipped"; | |
| resource.error = `Skipped: monitor limit reached (max ${maxMonitors})`; | |
| } |
| validate: async (config) => { | ||
| try { | ||
| const client = createBetterstackClient(config.apiKey); | ||
| await client.getMonitors(); | ||
| return { valid: true }; | ||
| } catch (err) { | ||
| return { | ||
| valid: false, | ||
| error: err instanceof Error ? err.message : String(err), | ||
| }; |
There was a problem hiding this comment.
validate() currently calls client.getMonitors(), which fetches all monitor pages due to requestAllPages(). That can be very slow/expensive for accounts with many monitors and makes validation heavier than necessary. Consider validating with a single lightweight request (e.g., fetch the first page only / a dedicated "me" endpoint / limit=1 style request) so validation only checks auth/permissions without pulling full datasets.
packages/api/src/service/import.ts
Outdated
| const monitorsPhase = summary.phases.find((p) => p.phase === "monitors"); | ||
| if (monitorsPhase && monitorsPhase.resources.length > 0) { | ||
| const maxMonitors = config.limits.monitors; | ||
| const existingMonitors = await db | ||
| .select() | ||
| .from(monitor) | ||
| .where(eq(monitor.workspaceId, config.workspaceId)) | ||
| .all(); | ||
| const remaining = maxMonitors - existingMonitors.length; |
There was a problem hiding this comment.
In addLimitWarnings(), the monitor limit check loads all existing monitors with .select().from(monitor)...all() just to compute a count. For larger workspaces this is unnecessarily heavy; prefer a COUNT(*) (or a minimal select({ id: monitor.id })) so the warning check stays cheap.
# Conflicts: # apps/dashboard/src/components/forms/components/form-import.tsx # packages/api/src/router/import.ts # packages/api/src/service/import.ts # packages/icons/src/index.tsx # packages/importers/package.json # packages/importers/src/index.ts
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
3 issues found across 13 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/importers/src/providers/betterstack/provider.ts">
<violation number="1" location="packages/importers/src/providers/betterstack/provider.ts:128">
P2: Sequential `await` inside a `for` loop creates an N+1 API call pattern. Each report triggers independent paginated HTTP requests that could run concurrently. With many reports this will be noticeably slow. Consider `Promise.all` (or batch in small groups if rate-limiting is a concern).</violation>
</file>
<file name="packages/api/src/service/import.ts">
<violation number="1" location="packages/api/src/service/import.ts:580">
P2: When `data.type` is `"monitor"` but `sourceMonitorId` is falsy, the fallback to `"static"` is bypassed, potentially inserting a component with `type: "monitor"` and `monitorId: null`. Move the `!data.monitorId` fallback outside the `sourceMonitorId` guard so any unresolved monitor-type component defaults to static.</violation>
</file>
<file name="packages/importers/src/providers/betterstack/mapper.ts">
<violation number="1" location="packages/importers/src/providers/betterstack/mapper.ts:22">
P2: 180 seconds (3 min) is mapped to `"5m"` but is equidistant from `"1m"`. This silently reduces monitoring frequency from every 3 minutes to every 5 minutes for migrated monitors. Consider mapping to `"1m"` instead, which keeps monitoring at least as frequent as the user configured (or introduce a `"3m"` periodicity if supported).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| phases.push({ | ||
| phase: "maintenances", | ||
| status: "completed", | ||
| resources: maintenanceResources, | ||
| }); | ||
| } else { |
There was a problem hiding this comment.
In the status-page branch, the provider never fetches/pushes a monitorGroups phase. This conflicts with the stated Better Stack mapping (monitor groups -> pageComponentGroup) and means monitor groups are only imported when no status page exists. Consider always importing monitorGroups (e.g., fetch once and push the phase regardless of status page presence) so group mappings are available consistently.
packages/api/src/service/import.ts
Outdated
| const remaining = maxMonitors - existingMonitors.length; | ||
|
|
||
| if (remaining <= 0) { | ||
| phase.status = "failed"; |
There was a problem hiding this comment.
When the workspace has no remaining monitor quota, this phase is marked as failed and returns early without marking individual resources. Because overall import status becomes failed if any phase fails, this can make the entire import fail even though other phases (page/components/incidents) could still succeed. Consider marking monitor resources as skipped (with an explanatory error) and setting the phase to completed/partial instead of failed so the rest of the import can proceed cleanly under plan limits.
| phase.status = "failed"; | |
| for (const resource of phase.resources) { | |
| resource.status = "skipped"; | |
| resource.error = `Skipped: monitor limit reached (${maxMonitors})`; | |
| } | |
| phase.status = computePhaseStatus(phase.resources); |
packages/api/src/service/import.ts
Outdated
| const monitorsPhase = summary.phases.find((p) => p.phase === "monitors"); | ||
| if (monitorsPhase && monitorsPhase.resources.length > 0) { | ||
| const maxMonitors = config.limits.monitors; | ||
| const existingMonitors = await db | ||
| .select() | ||
| .from(monitor) | ||
| .where( | ||
| and( | ||
| eq(monitor.workspaceId, config.workspaceId), | ||
| isNull(monitor.deletedAt), | ||
| ), | ||
| ) | ||
| .all(); | ||
| const remaining = maxMonitors - existingMonitors.length; |
There was a problem hiding this comment.
addLimitWarnings loads all existing monitors (.select().all()) just to compute the count. For large workspaces this is unnecessary work and can slow down preview/run. Prefer a COUNT(*) (or equivalent) query so you only fetch the count needed for limit calculations.
There was a problem hiding this comment.
2 issues found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/importers/src/providers/betterstack/provider.ts">
<violation number="1" location="packages/importers/src/providers/betterstack/provider.ts:153">
P2: Unbounded `Promise.all` over all reports can fire hundreds of concurrent HTTP requests to the Better Stack API, risking rate-limit (429) errors. Consider batching with a concurrency limit (e.g., `p-limit` or chunked iteration).</violation>
</file>
<file name="packages/importers/README.md">
<violation number="1" location="packages/importers/README.md:10">
P3: The Supported Providers table claims BetterStack imports "maintenances", but the BetterStack phases table below has no `maintenances` phase. This will confuse developers checking what the importer actually supports. Either add the missing phase row or remove "maintenances" from the summary.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Summary
Adds a Better Stack Uptime import provider to the
@openstatus/importerspackage, enabling users to migrate their monitoring setup from Better Stack into OpenStatus.This is the first importer that can import actual monitors (HTTP checks with URLs, frequency, regions) — the existing Statuspage importer only imports status page data.
What gets imported
monitortablepagetablepageComponentGrouptablepageComponentGrouptablestatusReport+statusReportUpdatetablesField mapping highlights
30s,1m,5m...)us,eu,as,au) mapped to specific Fly regions (iad,fra,sin,syd)started_at,acknowledged_at,resolved_at) converted to synthetic status report updates (investigating→identified→resolved)pagination.nextlinksChanges
New files (10)
packages/importers/src/providers/betterstack/— Full provider implementation:api-types.ts— Zod schemas for Better Stack API responsesclient.ts— HTTP client with Bearer auth and JSON:API paginationmapper.ts— 9 transformation functions (monitor, frequency, regions, status page, incidents, etc.)provider.ts— 5-phase import orchestrationindex.ts— Barrel exportsfixtures.ts— Mock data (3 monitors, 1 group, 1 status page, 2 sections, 3 incidents)client.test.ts— 8 tests (endpoints, auth, pagination, errors)mapper.test.ts— 25 tests (all mapping functions with edge cases)provider.test.ts— 7 tests (validate, phases, filtering)packages/icons/src/betterstack.tsx— Better Stack icon componentpackages/importers/README.md— Package documentation with architecture, pipeline flow, and "adding a new provider" guideModified files (6)
packages/importers/src/index.ts— Registerbetterstackprovider inIMPORT_PROVIDERSpackages/importers/package.json— Add./betterstackand./betterstack/fixturesexport pathspackages/icons/src/index.tsx— Export Better Stack iconpackages/api/src/service/import.ts— AddwriteMonitorsPhase(),createProvider()routing, monitor limit warnings,monitors/monitorGroups/sectionsphase casespackages/api/src/router/import.ts— Accept"betterstack"provider,betterstackStatusPageId,includeMonitorsoptionapps/dashboard/src/components/forms/components/form-import.tsx— Better Stack radio button, provider-specific fields, monitors toggleTest plan
bun testinpackages/importers/— 75 tests pass (40 new + 35 existing)